Skip to content

[#328] Improve royalty claim UX: post-claim state + claimed history#332

Merged
realproject7 merged 2 commits intomainfrom
task/328-royalty-claim-ux
Mar 18, 2026
Merged

[#328] Improve royalty claim UX: post-claim state + claimed history#332
realproject7 merged 2 commits intomainfrom
task/328-royalty-claim-ux

Conversation

@realproject7
Copy link
Copy Markdown
Owner

Summary

  • Post-claim state: Claim button disabled with "Claimed" text after success; balance display reflects 0
  • Success message: Shows Claimed X WETH — tx: 0xabc...def with clickable BaseScan link
  • Cumulative claimed: Reads claimed from getRoyaltyInfo (2nd return value); displays (claimed: X WETH so far) when > 0
  • Auto-reset: Returns to idle state on next refetch cycle so updated balance shows cleanly

Single file change: src/components/ClaimRoyalties.tsx

Fixes #328

Test plan

  • npm run build passes
  • npm run typecheck passes
  • Verify post-claim button shows "Claimed" and is disabled
  • Verify tx link is clickable and points to BaseScan
  • Verify "claimed so far" appears only when cumulative claimed > 0
  • Verify state resets to idle after refetch interval

- Post-claim: button disabled with "Claimed" text, balance reflects 0
- Success message shows claimed amount with clickable BaseScan tx link
- Read cumulative claimed from getRoyaltyInfo; show "claimed so far"
  suffix when > 0
- Auto-reset to idle state on next data refetch after claim

Fixes #328

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

T2b APPROVE

Single-file change, all acceptance criteria met:

  • Post-claim: button disabled with "Claimed" text ✅
  • Success message: Claimed X WETH — tx: 0xabc...def with clickable BaseScan link ✅
  • Cumulative claimed: reads claimed from getRoyaltyInfo, shown only when > 0 ✅
  • Auto-reset: queryFn resets txState to idle on next refetch cycle ✅
  • "No royalties yet" message suppressed when totalClaimed > 0

Minor style note (non-blocking): setting React state inside queryFn is unconventional — a useEffect watching txState would be more idiomatic. Works correctly as-is.

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The UI changes are close, but the success state is cleared immediately by the manual refetch path, so the user does not reliably get the required disabled Claimed state and tx-link confirmation after a successful claim.

Findings

  • [high] The component resets txState to idle inside the query function, and executeClaim() immediately calls refetch() after setting txState to done.
    • File: src/components/ClaimRoyalties.tsx:41
    • Suggestion: Do not mutate component state inside queryFn. Keep the success state visible after a successful claim, then reset on a later refetch cycle or in an explicit effect tied to fresh data. As written, the success state can disappear as soon as the manual refetch() resolves, which undercuts the acceptance criteria for a disabled Claimed button and visible success message/tx link.

Decision

Request changes. The post-claim confirmation state needs to persist long enough for the user to actually see it before the component returns to idle.

Remove inline txState reset from queryFn and immediate refetch()
call from executeClaim. Instead, use a ref+effect to detect the
next automatic royaltyInfo refetch (30s interval) and reset to
idle then. This preserves the "Claimed" success state with tx link
until fresh data arrives.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

The follow-up fixes the post-claim state regression by keeping the success state out of the query function and only resetting after a later data refresh. The UX now matches issue #328 closely.

Findings

  • [low] The prior blocker is resolved: txState is no longer mutated inside queryFn, and the component now waits for a later royalty-info refresh before returning from done to idle.
    • File: src/components/ClaimRoyalties.tsx:62
    • Suggestion: None.

Decision

Approve. The component now preserves the disabled Claimed state and tx link after success, shows cumulative claimed amount when present, and local npm run build plus npm run typecheck both pass.

@realproject7 realproject7 merged commit 053cbfd into main Mar 18, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve royalty claim UX: post-claim state + claimed history

2 participants